Implement this_client() #456

ranfdev · 2023-11-27T22:27:32Z

fixes #87.

One question though: this currently works well for a struct implementing a single interface.
If a single struct implements multiple interfaces, there will be multiple this_client implementations, one for each interface.
I'm not sure how this is going to handle multiple interfaces...

For example:

struct Calculator{}
impl calculator_add::Server for Calculator {
  fn add(&mut self, params, results) {...} 
}
impl calculator_sub::Server for Calculator {
  fn sub(&mut self, params, results) {
    let client = calculator_sub::Server::this_client(self); // Works perfectly.
    let client = calculator_add::Server::this_client(self); // OUCH, we are trying to get a `calculator_add::Client` from a `calculator_sub::Client`.
  } 
}

If the two interfaces share the same ClientHook anyway, this is fine. Do they? Else, it's going to be a problem.

dwrensha · 2023-11-29T00:49:35Z

capnp-rpc/src/local.rs

+
+                // We need to provide a way to get the corresponding ClientHook of a Server.
+                // Passing the ClientHook inside `Params` would require a lot of changes, breaking compatiblity
+                // with existing code and ruining the developer experience, because `Params` would end up containing more generics.


What if this_client() returned an untyped capability -- perhaps just a Box<dyn ClientHook>? That would not require any more generics, would it?
(It looks to me like that's how capnproto-c++ implements thisCap().)

Having a params.this_client() returning Box<dyn ClientHook> would be possible but it has the following drawbacks:

the user needs to manually cast the client.

params is passed by value, and can be moved around and stored eventually for a 'static lifetime. That means params would need to own a copy of the current client, requiring a Box allocation for each method call. Unless we store a Weak reference to the ClientHook, but then params.this_client() needs to return an Option<T>, because the params we own may reference a dead ClientHook.

Returning an Option<Box<dyn ClientHook>> isn't great compared to a fully typed calculator::op_add::Client

An alternative API which doesn't require unsafe nor a static variable is
params.client_for(self). (Where self is actually &mut self, because we are inside a method call).

By requiring &mut self as a parameter, we restrict the ability to get a client while we are inside the method call, and we also have the type information required to return a typed Client.

EDIT: no, this API doesn't have enough type information to return a typed client... But at least it can return directly a Box<dyn ClientHook> and doesn't need to return an Option<Box<dyn ClientHook>>

dwrensha · 2024-01-12T13:45:18Z

I implemented an alternative in #473.

Implement this_client()

ac47938

dwrensha reviewed Nov 29, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement this_client() #456

Implement this_client() #456

ranfdev commented Nov 27, 2023

dwrensha Nov 29, 2023

ranfdev Nov 29, 2023

ranfdev Nov 29, 2023 •

edited

Loading

dwrensha commented Jan 12, 2024

Implement this_client() #456

Are you sure you want to change the base?

Implement this_client() #456

Conversation

ranfdev commented Nov 27, 2023

dwrensha Nov 29, 2023

Choose a reason for hiding this comment

ranfdev Nov 29, 2023

Choose a reason for hiding this comment

ranfdev Nov 29, 2023 • edited Loading

Choose a reason for hiding this comment

dwrensha commented Jan 12, 2024

ranfdev Nov 29, 2023 •

edited

Loading